Conversation
stackotter
left a comment
stackotter
left a comment
There was a problem hiding this comment.
Thanks for the PR! Just a few small requested changes, but looks good for the most part
|
Requires #31 |
stackotter
left a comment
stackotter
left a comment
There was a problem hiding this comment.
Thanks for addressing those changes. Looking pretty good now, just got a few more requests. Thanks for catching and fixing the floating point issue too 🙏
| } | ||
|
|
||
| /// Raw representable conformance is only synthesized when using integer literals (eg 1), not float literals (eg 1.0). | ||
| func previousValue() -> Int? { |
There was a problem hiding this comment.
Move this helper function to the top of the function to make things read a bit nicer.
| case .string: .inferredRawValue(.init(value: "\"\(raw: _syntax.name.text)\"" as ExprSyntax)) | ||
| case .character: nil // Characters cannot be inferred | ||
| case .integer, .floatingPoint: .inferredRawValue(.init(value: "\(raw: (previousValue() ?? -1) + 1)" as ExprSyntax)) |
There was a problem hiding this comment.
Move the values (after the colons) onto new lines to reduce line length and make things a bit easier to skim
| public var value: EnumCaseValue? | ||
|
|
||
| /// Helper that gets the associated values from `EnumCase.value` or returns an empty array. | ||
| public var associatedValues: [EnumCaseAssociatedValueParameter] { |
There was a problem hiding this comment.
I feel like this should be renamed to make it clear that it's the types of the associated values and not actual values.
There was a problem hiding this comment.
Maybe associatedValueParameters? (given that they include labels as well)
| } | ||
|
|
||
| /// Helper that gets the raw or inferred raw value text, eg "value", 1, or 1.0. | ||
| public var rawValueText: String? { |
There was a problem hiding this comment.
Is this essentially equivalent to rawValue?.description with a bit of added normalisation? I feel like in situations where you'd use this, you don't need the normalisation anyway, so rawValue?.description would suffice (in which case this property shouldn't be necessary).
There was a problem hiding this comment.
I had no idea that was possible so just my inexperience with swift syntax showing.
|
|
||
| /// Enum raw values can be strings, characters, or any of the integer or floating-point number types. | ||
| public enum EnumRawRepresentableType { | ||
| case string(syntax: IdentifierTypeSyntax) |
There was a problem hiding this comment.
Every case has the same associated value, so I feel it may be better to turn this enum into a struct with a nested Kind enum (containing these cases but without associated values) alongside kind and identifier fields.
2 participants
Previously we could already get the raw value of a case with
EnumCase.valueand now that can also return a new case called inferredRawValue.EnumCaseValue.rawValueandEnumCaseValue.inferredRawValuehave the same associated value type so you can do:This PR only adds raw representable checking when it is the first inherited type, which is the simplest case. It does not check eg
typealias RawValue = String.